Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update API client #1032

Merged
merged 22 commits into from
May 24, 2021
Merged

Update API client #1032

merged 22 commits into from
May 24, 2021

Conversation

FSM1
Copy link
Contributor

@FSM1 FSM1 commented May 18, 2021 •
edited
Loading

Remove references to the soon to be deprecated files/ api and replace these with the bucket/{bucket id} apis

Fixed a bunch of routing problems by using encodeURIComponent

Updated react-pdf and used the webpack entry point

@render
Copy link

render bot commented May 18, 2021

Your Render PR Server URL is https://files-ui-stage-pr-1032.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-c2hr5jr2v8p851phcg50.

@FSM1 FSM1 self-assigned this May 18, 2021
@FSM1 FSM1 added Release: Major only for PROD release PR Type: Maintenance Added to issues and PRs when a change is for repository maintenance , such as CI or linter changes. and removed Release: Major only for PROD release PR labels May 18, 2021
@FSM1 FSM1 marked this pull request as ready for review May 20, 2021 16:01
FSM1 and others added 2 commits May 21, 2021 07:11
@FSM1 @tanmoyAtb
Apply suggestions from code review
331deef 
Co-authored-by: Tanmoy Basak Anjan <tanmoy3399@gmail.com>
@FSM1 @tanmoyAtb
Update packages/files-ui/src/Utils/pathUtils.ts
d96583b 
Co-authored-by: Tanmoy Basak Anjan <tanmoy3399@gmail.com>
tanmoyAtb
tanmoyAtb requested changes May 21, 2021
View reviewed changes
Copy link
Contributor

@tanmoyAtb tanmoyAtb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one hell of a refactor. I am still trying to wrap my head around everything. I have a better understanding of the filebrowser contexting now.

I noticed these functionalities breaking.

  • Rename not working (mainly because the renameItems wants CIDs, but the browser is sending a path)

  • In grid view, folder navigation not working (viewFolder prop not passed)

  • Download from menu options is breaking

  • Could not load file info (this might be API problem)

  • In Bin view, recover file doesn't trigger

  • I'm not getting any search results in the dropdown. but getting them in browser view.

we can always break some of these problems into separate issues if the changes are too big,

Note: I noticed we are using the name FilesContext but also useDrive, is this intentional design? I got a little confused with them

RyRy79261
RyRy79261 approved these changes May 21, 2021
View reviewed changes
Copy link
Contributor

@RyRy79261 RyRy79261 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work 🔥

@FSM1
Copy link
Contributor Author

FSM1 commented May 21, 2021

This is one hell of a refactor. I am still trying to wrap my head around everything. I have a better understanding of the filebrowser contexting now.

I noticed these functionalities breaking.

  • Rename not working (mainly because the renameItems wants CIDs, but the browser is sending a path)
  • In grid view, folder navigation not working (viewFolder prop not passed)
  • Download from menu options is breaking
  • Could not load file info (this might be API problem)
  • In Bin view, recover file doesn't trigger
  • I'm not getting any search results in the dropdown. but getting them in browser view.

we can always break some of these problems into separate issues if the changes are too big,

Note: I noticed we are using the name FilesContext but also useDrive, is this intentional design? I got a little confused with them

Thanks for checking up on these. I will rather just fix up all these glitches now... I dont think that any of those should take me too long to sort out...

Tbaut
Tbaut reviewed May 21, 2021
View reviewed changes
Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow there's a huge amount of goodness in this PR!! I went through the code in details, will test thoroughly after the fix for everything Tanmoy mentioned.

packages/files-ui/src/Components/Modules/DownloadProgressModals/index.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/FileBrowsers/BinFileBrowser.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/FileBrowsers/BinFileBrowser.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/FileBrowsers/CSFFileBrowser.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/FileBrowsers/CSFFileBrowser.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/FileBrowsers/MoveFileModal.tsx Show resolved Hide resolved
packages/files-ui/src/Components/Modules/UploadProgressModals/index.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/FileBrowsers/FileInfoModal.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Contexts/FilesContext.tsx Show resolved Hide resolved
packages/files-ui/src/Contexts/FilesContext.tsx Outdated Show resolved Hide resolved
@FSM1 FSM1 requested a review from tanmoyAtb May 23, 2021 20:31
@FSM1 FSM1 added Release: Major only for PROD release PR Status: Review Needed 👀 Added to PRs when they need more review labels May 23, 2021
@FSM1
Copy link
Contributor Author

FSM1 commented May 23, 2021

This is one hell of a refactor. I am still trying to wrap my head around everything. I have a better understanding of the filebrowser contexting now.

I noticed these functionalities breaking.

  • Rename not working (mainly because the renameItems wants CIDs, but the browser is sending a path)
  • In grid view, folder navigation not working (viewFolder prop not passed)
  • Download from menu options is breaking
  • Could not load file info (this might be API problem)
  • In Bin view, recover file doesn't trigger
  • I'm not getting any search results in the dropdown. but getting them in browser view.

we can always break some of these problems into separate issues if the changes are too big,

Note: I noticed we are using the name FilesContext but also useDrive, is this intentional design? I got a little confused with them

all of these items seem to be fixed

tanmoyAtb
tanmoyAtb approved these changes May 24, 2021
View reviewed changes
Copy link
Contributor

@tanmoyAtb tanmoyAtb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is 🔥 of a refactor!

@FSM1 FSM1 merged commit 18e62a8 into dev May 24, 2021
@FSM1 FSM1 deleted the mnt/update-api-client-1008 branch May 24, 2021 18:02
@FSM1 FSM1 mentioned this pull request May 27, 2021
Merged
FSM1 added a commit that referenced this pull request May 31, 2021
@FSM1
Merge pull request #1059 from ChainSafe/dev
530b582 
* Fix upload (#1010)
* Bulk operations for Bin (#1017)
* Set the base for Cypress tests and automation (#1016)
* Update Send Feedback Link (#1024)
* only enable logging in non-mainnet env (#1020)
* Optimize tests (#1027)
* fix (#1030)
* File browser context provider (#1026)
* work on selections  (#1029)
* Remove release drafter for now (#1038)
* Update Readme for tests (#1036)
* Delete release_drafter.yml (#1039)
* Bulk DND Move files (#1028)
* Test file upload (#1035)
* Selection removal preventions (#1037)
* Theme selection UI fix (#1033)
* Update API client (#1032)
* make it light and the files blue (#1049)
* React-PDF Worker fixes (#1052)
* Store screenshots and video of failing tests (#1054)
* fix test classes and add rename test (#1055)
* Hide date for folders (#1060)
* s/testId/testid (#1064)
* folder path in route fixed (#1066)
* Resolve File Browser race condition (#1069)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tbaut Tbaut Tbaut left review comments

@tanmoyAtb tanmoyAtb tanmoyAtb approved these changes

@RyRy79261 RyRy79261 RyRy79261 approved these changes

Assignees

@FSM1 FSM1

Labels
Release: Major only for PROD release PR Status: Review Needed 👀 Added to PRs when they need more review Type: Maintenance Added to issues and PRs when a change is for repository maintenance , such as CI or linter changes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@FSM1 @RyRy79261 @tanmoyAtb @Tbaut @actions-user